Skip to content

Conversation

emaste
Copy link
Member

@emaste emaste commented Mar 22, 2024

libc++ will drop support for Clang 16 before long.

@emaste emaste requested a review from a team as a code owner March 22, 2024 18:07
@llvmbot llvmbot added the libc++ libc++ C++ Standard Library. Not GNU libstdc++. Not libc++abi. label Mar 22, 2024
@llvmbot
Copy link
Member

llvmbot commented Mar 22, 2024

@llvm/pr-subscribers-libcxx

Author: Ed Maste (emaste)

Changes

libc++ will drop support for Clang 16 before long.


Full diff: https://github.com/llvm/llvm-project/pull/86320.diff

1 Files Affected:

  • (modified) libcxx/utils/ci/buildkite-pipeline.yml (+2-2)
diff --git a/libcxx/utils/ci/buildkite-pipeline.yml b/libcxx/utils/ci/buildkite-pipeline.yml
index e42262620d5fb0..31e794e67d330d 100644
--- a/libcxx/utils/ci/buildkite-pipeline.yml
+++ b/libcxx/utils/ci/buildkite-pipeline.yml
@@ -209,8 +209,8 @@ steps:
   - label: FreeBSD 13 amd64
     command: libcxx/utils/ci/run-buildbot generic-cxx23
     env:
-      CC: clang16
-      CXX: clang++16
+      CC: clang17
+      CXX: clang++17
     agents:
       queue: libcxx-builders
       os: freebsd

Copy link
Member

@mordante mordante left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks! LGTM!

@Zingam
Copy link
Contributor

Zingam commented Mar 22, 2024

Can we have C++26 mode enabled to be able to test the new features?

@mordante
Copy link
Member

Can we have C++26 mode enabled to be able to test the new features?

Good point! Thanks! @emaste can you change that too?

@emaste
Copy link
Member Author

emaste commented Mar 22, 2024

Observed the following failures on the FreeBSD builder with this change:

********************
Failed Tests (5):
  llvm-libc++-shared.cfg.in :: libcxx/selftest/modules/std-and-std.compat-module.sh.cpp
  llvm-libc++-shared.cfg.in :: libcxx/selftest/modules/std-module.sh.cpp
  llvm-libc++-shared.cfg.in :: libcxx/selftest/modules/std.compat-module.sh.cpp
  llvm-libc++-shared.cfg.in :: std/modules/std.compat.pass.cpp
  llvm-libc++-shared.cfg.in :: std/modules/std.pass.cpp

Typical error:

# | In file included from /home/buildkite/.buildkite-agent/builds/freebsd-test-1/llvm-project/libcxx-ci/build/generic-cxx23/modules/c++/v1/std.cppm:227:
# | /home/buildkite/.buildkite-agent/builds/freebsd-test-1/llvm-project/libcxx-ci/build/generic-cxx23/modules/c++/v1/std/cfenv.inc:16:14: error: using declaration referring to 'feclearexcept' with internal linkage cannot be exported
# |    16 |   using std::feclearexcept;
# |       |              ^
# | /usr/include/fenv.h:267:1: note: target of using declaration
# |   267 | feclearexcept(int __excepts)
# |       | ^

@mordante
Copy link
Member

It seems FreeBSD's libc is not standard compliant.
/usr/include/fenv.h:266

__fenv_static inline int
feclearexcept(int __excepts)

and __fenv_static is defined as static. This means feclearexcept has internal linkage. This is not allowed by the C standard and C++ modules can't export named declarations with internal linkage. So this is probably something to be fixed in FreeBSD.

For now you can disable the module tests by adding or "__FreeBSD__" in compilerMacros(cfg) in this test feature.
https://github.com/llvm/llvm-project/blob/main/libcxx/utils/libcxx/test/features.py#L279

@DimitryAndric
Copy link
Collaborator

Would extern inline work better here? Or is that also incompatible? (As far as I can see both the C and C++ standard just refer to this function as int feclearexcept(int excepts);.

@mordante
Copy link
Member

That's indeed how they are written in the standard. But per C11 7.1.2 Standard headers/6
Any declaration of a library function shall have external linkage. I did not check older C standards.

Since it seems most (all?) declarations in this headers are definitions just inline should work. However I expect the developers of FreeBSD's libc know how to fix this and why they use something different at the moment.

@emaste
Copy link
Member Author

emaste commented Mar 25, 2024

For now you can disable the module tests

I'll give that a try now

emaste added 2 commits March 25, 2024 15:26
FreeBSD's libc is noncompliant as noted in discussion in llvm#86320.
libc++ will drop support for Clang 16 before long.
Copy link

✅ With the latest revision this PR passed the Python code formatter.

Copy link

✅ With the latest revision this PR passed the C/C++ code formatter.

@emaste
Copy link
Member Author

emaste commented Mar 25, 2024

FreeBSD bug 277958 submitted for libc: https://bugs.freebsd.org/bugzilla/show_bug.cgi?id=277958

@emaste
Copy link
Member Author

emaste commented Mar 25, 2024

@emaste emaste merged commit 4fc8df9 into llvm:main Mar 25, 2024
@emaste
Copy link
Member Author

emaste commented Mar 25, 2024

Hrm, it's a bit unfortunate that the only option is "squash and merge" and any evidence of 490441c as a separate change is lost.

@emaste
Copy link
Member Author

emaste commented Mar 25, 2024

Can we have C++26 mode enabled to be able to test the new features?

Where do we enable it?

- group: ':freebsd: FreeBSD'
steps:
- label: FreeBSD 13 amd64
command: libcxx/utils/ci/run-buildbot generic-cxx23
Copy link
Contributor

@H-G-Hristov H-G-Hristov Mar 26, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@emaste
Copy link
Member Author

emaste commented Mar 26, 2024

Can we have C++26 mode enabled to be able to test the new features?

OK, in #86658 I tested switching to generic-cxx26 and it's still green. However, I don't think there are any other C++26 builders yet and I don't want FreeBSD to be the only one - I worry that if FreeBSD fails it will be assumed to be a FreeBSD issue rather than a C++26 issue.

@mordante
Copy link
Member

As mention in the other PR we already use C++26. However there used in GitHub actions and not on Buildkite.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
libc++ libc++ C++ Standard Library. Not GNU libstdc++. Not libc++abi.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants